feat(native): add menu-bar popover and background attention - #682

📋 PR Overview

🔬 Coverage

Generated by 🚫 dangerJS against e139f0e

🎨 Storybook preview

Open Storybook preview

Updated for e139f0e

🧭 Story changes

Compared to fix/helper-recovery (snapshot diff at story level):

✏️ Changed stories (2)

• Settings/DeveloperTab › Pinned To Past Version
• Settings/DeveloperTab › Unpinned

💡 Update snapshots ↗ to regenerate baselines and open a PR.

⚠️ Detected UI changes (5)

These stories' HTML snapshots changed. I've added screenshots + links to the changed stories below. Review them carefully then accept the changes to regenerate baselines and include them in this PR:

Flows/Evolve › Playground

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › Evolving With Error Event

Flows/Evolve › 3. Review (changes generated)

Accept UI changes

• Click here to accept these changes

Alternatively, you can run bun run test:update-snapshots locally to re-generate the baselines and then push the changes to this PR.

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Menu-bar popover and background attention for developer alpha

Product decisions in this change

1. The feature uses a deprecated macOS notification API to avoid the permission dialog that the supported replacement requires.

The deprecated API delivers a better first-run experience: no authorization sheet appears when the user opts in to the popover. The tradeoff is that Apple can remove the API at any OS version without a compile-time error, silently stopping all popover notifications. No migration plan, code comment, or linked issue appears in this PR. The developer gate limits current exposure. The concern grows when the feature approaches general availability, at which point removal without a plan leaves users with no notification path and no warning.

2. Silent notifications (changes ready, build finished) leave no persistent ambient signal after the user dismisses them, because the floating preview indicator is disabled in popover mode.

Silent delivery for non-blocking completions is the right default. The combination with a disabled preview indicator creates a coverage gap: in control mode the floating corner indicator reminds the user that uncommitted changes exist. In popover mode that indicator is off, and a dismissed silent notification leaves nothing. A user who completes an AI request, dismisses the notification while switching tasks, and returns an hour later has no visible cue that their changes are waiting.

The PR author acknowledges this and defers a tray-badge or icon-color solution to wider rollout. The developer-only gate keeps the impact small now.

Option	What it gives users	What it costs	Effort to change later
Current approach (no indicator)	Simpler tray icon	Dismissed silent notice leaves no reminder of pending work	Medium: new tray badge or icon-color logic needed
Tray badge or icon color change	Persistent signal survives notification dismissal	Small added visual complexity in the menu bar	Low: additive, does not change existing behavior
Re-enable preview indicator	Full parity with control mode for pending-change awareness	Floating window conflicts with the popover ambient-operation model	Low: gated by the existing flag

3. Active work (AI requests, builds, restores) continues running after the user dismisses or hides the popover.

This is the core product promise of the feature. A popover that cancels work on hide would be worse than the existing control window. The implementation keeps the view mounted across hide cycles, so the user returns to exactly the screen they left. The decision is clearly correct and the test suite covers it thoroughly.

4. Starting a new AI request or build clears all pending completion and failure notifications from the previous work session.

Clearing stale notifications at the start of new work prevents a confusing accumulation of outdated results. The narrow risk is a race: a user who sees a ChangesReady notification, begins reviewing it, and simultaneously triggers a new request will see the notification disappear from the notification center. The result they were about to act on is gone permanently. For a developer tool in alpha this is an acceptable edge case. At general availability, a confirm-before-clear or superseded-label policy for unreviewed results would avoid the data-loss feel.

5. The popover window is fixed at 800x600 with no user resize.

A fixed size is standard for tray popovers and keeps positioning logic predictable on multi-display setups. The PR includes a native test confirming that a 401-line diff, a 120-section reply, and 500 lines of build console output all remain scrollable with primary controls reachable at that size. The constraint is appropriate for the current scope.

6. Apply finalization errors are now shown to the user instead of being silently written to the console.

The previous behavior swallowed finalization errors with a console log, leaving the user with no indication that the Save step failed. Surfacing the error lets the user take corrective action. This change is correct in both window modes and is a straightforward improvement.

7. The Escape key routes through a fixed priority stack (Radix dialogs first, then app-owned overlays, then the popover dismiss) rather than relying on listener registration order.

Priority-ordered routing matches the visual depth of each layer and makes the behavior predictable regardless of how React mounts components. Radix layers handle their own events before the app dispatcher sees them, which is the correct split between library and application responsibilities. The test coverage for edge cases (IME composition, default-prevented events, native close tokens, delayed dismissal) is thorough.

8. In popover mode, the tray menu removes the "Open nixmac" menu item and uses a left-click on the tray icon to toggle the window directly.

Left-click-to-toggle is the standard model for tray popovers (Dropbox, 1Password, and similar apps all use it). Removing the redundant "Open nixmac" menu item declutters the right-click menu. A user migrating from control mode may briefly look for the menu item, but the left-click behavior is discoverable immediately on first use.

9. PostHog-backed and local-only feature flags share one developer settings panel, with per-flag description text to distinguish them.

Putting all developer overrides in one place is convenient. The concern is that the "PostHog default" reset label means different things depending on the flag: for PostHog-backed flags it reverts to the server-side value; for local-only flags it reverts to the app default. The defaultLabel field mitigates this by showing "Default (control)" for local-only flags. The distinction is still easy to miss for a developer who skips the description text. A visual separator or section header between the two categories would make the difference obvious at a glance.

10. The mode (popover versus control window) is a launch-time setting that requires a full restart to change.

A launch-time mode avoids the complexity of hot-switching two fundamentally different window management strategies, including tray behavior, position tracking, notification backend, and indicator windows. The restart requirement is clearly labeled in the developer settings. For a developer alpha with a small audience this is an appropriate tradeoff.

Since the last review

• Addressed by explanation: All attention notifications reopen to the same last-visible screen regardless of which event triggered them. (PR conversation: "Routing to a particular diff or blocker remains a separate follow-up, including how old notifications interact with newer work and unsaved edits.")
• Still open: The popover notification backend uses the deprecated NSUserNotificationCenter API with no documented migration plan. (attention.rs retains the #[allow(deprecated)] macos module and uses NSUserNotificationCenter throughout; no migration comment or linked issue appears in the diff.)
• Still open: ChangesReady and BuildSucceeded are silent background notifications and the preview indicator is disabled in popover mode, leaving no persistent ambient signal after a dismissed notice. (attention.rs sets request_attention: false for ChangesReady and BuildSucceeded; peek.rs gates preview-indicator creation and updates on shows_detached_indicators(), which returns false in popover mode.)
• Addressed by explanation: Is a persistent ambient indicator planned before the popover flag exits the developer gate? (PR conversation: "A persistent menu-bar pending-change indicator remains follow-up scope for wider rollout." The feature remains developer-only with an explicit restart gate.)
• Still open: Is there a documented migration plan from the deprecated NSUserNotificationCenter to UNUserNotificationCenter before the feature reaches general availability? (No migration plan, code comment, or linked issue appears in this PR or in attention.rs.)
• Addressed in code: When a new AI request starts, clear_work dismisses any pending notifications from the previous session. Is this the intended behavior? (commands/evolve.rs calls crate::attention::clear_work(&app) at the top of run_evolve; state/rebuild_status.rs calls it in record_start. The pattern is consistent across both entry points, confirming the choice is deliberate.)

Open questions

• Before the popover flag exits the developer gate, will a persistent ambient indicator (tray badge or icon state) be in place to cover the gap left by the disabled preview indicator?

• Is there a tracked issue or written plan to migrate from the deprecated notification API before the feature reaches general availability, so that a future macOS release does not silently break all popover notifications?

• When a new request begins and clears a pending ChangesReady notification that the user has not yet acted on, should the user see any acknowledgment that the previous result has been superseded?

Recommendation

Ship with changes
The feature is solidly built and correctly gated behind a developer-only, restart-required flag, which limits all current user-facing risk. Two prior concerns remain unaddressed: the deprecated notification API has no documented migration path, and silent notifications leave no ambient signal after dismissal. Neither blocks the developer alpha, but both need a written resolution plan before the feature exits the gate. Adding a code comment or linked issue for the API migration path, and a one-line note in the developer settings description about the missing ambient indicator, would close both gaps without requiring new implementation work in this PR.

Thanks, I checked each open question against the branch:

• History / Filesystem: Clicking away hides the window without resetting the view. Native checks on cbc6b61 confirm History returns on reopen, while native Close dismisses History first and leaves the popover open. Standalone Filesystem and the editable NixEditor have no reachable UI entry point in this build, so their retention coverage remains automated.
• Pending changes across Spaces: There is no persistent replacement for the floating preview indicator in this opt-in. A persistent menu-bar pending-change indicator remains follow-up scope for wider rollout.
• Notification clicks: They reopen the retained view. Both full History restore success and finalization-error notices were delivered and activated successfully. Routing to a particular diff or blocker remains a separate follow-up, including how old notifications interact with newer work and unsaved edits.
• 600px height: Native checks at 800 x 600 pass for a 401-line diff, a 120-section reply and 500 lines of build-console fixture data. Content scrolls and the primary controls remain reachable. Diff/reply scroll positions survive hide/reopen. Both existing displays and a controlled unavailable-tray fallback were checked; other screen sizes and display disconnection were not part of this pass.
• Notification preferences: There is no per-category selector here. An action-needed-only option for wider rollout would retain questions, blockers and failures while silencing routine completion notices.
• Notification API migration: This PR retains the existing native notification API; migration is not included here. Before wider rollout, I would move to the supported notification backend and recheck permissions, delivery and click-to-reopen behavior. The feature remains a developer opt-in.
• Clearing earlier notices: Starting new work currently clears earlier work notifications and their tracking. It preserves working-tree changes and History. I would keep that behavior for this opt-in; a superseded-result treatment belongs with the wider-rollout indicator design.

Failure copy is updated: evolution failure says “Your request stopped with an error”; the shared build/restore path says “Build or restore failed” with recovery guidance. Finalization errors retain the distinction that changes may already be active. The full History restore path now also notifies on finalization success or failure.

The feature stays a local developer opt-in with an explicit restart requirement. A dedicated preference and migration of the local override belong with wider rollout. Normal-window shortcuts and resizing passed their native checks; dismissing the popover keeps active work running.

The popover implementation at cbc6b6146 passed 1,412 Rust tests (14 ignored), 454 frontend tests, TypeScript, the frontend build, strict workspace Clippy and all 12 GitHub checks. Native checks use an isolated debug/e2e app with mocked system activation; restore bookkeeping is real. The forced first-lookup failure uses a separate instrumented startup fixture. The PR checklist records the exact scope.